Skip to content

fix: stop mail loops from bounces and autoresponders - #3628

Merged
aerodeval merged 7 commits into
frappe:developfrom
aerodeval:fix/bounce-notification-loop
Sep 3, 2026
Merged

fix: stop mail loops from bounces and autoresponders#3628
aerodeval merged 7 commits into
frappe:developfrom
aerodeval:fix/bounce-notification-loop

Conversation

@aerodeval

@aerodeval aerodeval commented Jul 28, 2026

Copy link
Copy Markdown
Member

What

Inbound mail is only screened with the X-Auto-Generated header, which nothing but helpdesk's own acknowledgement emails set. Real bounces and out-of-office replies pass straight through into ticket creation, where they can start a mail loop with the sending MTA.

Once a machine-generated mail becomes a ticket or a communication, three separate paths can reply to it:

  1. New ticket ackHD Ticket.after_insert sends the acknowledgement to raised_by, i.e. the address that just bounced.
  2. Portal ticket threading — when the mail threads onto a ticket created from the portal, frappe CCs the parent doc's owner (mail_cc) on every inbound mail — the same dead address.
  3. enable_auto_reply — an account with auto-reply on answers mailer-daemon directly.

Each reply bounces again, which creates another inbound mail, and so on.

Changes

All in helpdesk/overrides/email_account.py:

auto_generated_reason(msg) identifies machine-generated mail by the standard markers instead of relying on our own header. Checked in this order:

Marker Standard Typical sender Verdict
X-Auto-Generated helpdesk's own header our ack / feedback mail looping back in 🚫 parked
Content-Type: multipart/report; report-type=delivery-status RFC 3464 Gmail / Postfix bounce (DSN) 🚫 parked + "Delivery failed" ticket comment
Return-Path: <> (null envelope sender) RFC 5321 §6.1 bounce whose report part got stripped 🚫 parked + ticket comment
Auto-Submitted: auto-generated (or any value other than no / auto-replied) RFC 3834 monitoring alerts, cron mail, quarantine notices 🚫 parked (+ "Auto-reply received" comment when it belongs to a ticket)
Auto-Submitted: auto-replied RFC 3834 out of office, read receipts ✅ allowed — threads onto its ticket
Auto-Submitted: no (parameters tolerated, e.g. no; owner=...) RFC 3834 explicitly human-sent mail ✅ allowed
no markers ordinary customer mail ✅ allowed

Why auto-replied passes: an out-of-office should reach agents on the ticket, and it cannot sustain a loop the way a bounce can — it only threads (no new ticket, so no acknowledgement goes out), and auto-responders rate-limit themselves to one reply per sender. Bounces and auto-generated feeds have no such limit, which is why they stay parked.

The bounce markers are checked before Auto-Submitted, so a DSN that also carries Auto-Submitted: auto-replied (Gmail's do) is still parked, with the recorded reason naming the dead address rather than a mere autoresponder.

Parked, not dropped. Matches go to handle_bad_emails(uid, message, reason) and land in Unhandled Email with the reason recorded, instead of being silently dropped by a bare continue. The framework version of handle_bad_emails only records for IMAP accounts, so it is overridden without the use_imap gate — POP3 and Frappe Mail drops leave the same trace, and a misclassified customer mail is always recoverable.

Agents still see what happened. Parking means bounces no longer thread onto tickets — an agent would never learn their reply went nowhere. When a parked mail belongs to an existing ticket (same In-Reply-To/References lookup the threading code uses), notify_ticket_of_parked_mail leaves an internal comment there by Administrator:

  • bounce → "Delivery failed: the reply to this ticket could not be delivered to dead@example.com." (address taken from the DSN's Final-Recipient when present)
  • parked auto-generated reply → "Auto-reply received from postmaster@example.com."

Comments notify nobody (no @mention), customers never see them, and a comment failure only logs — it cannot break the mail pull. No dedupe: one comment per parked mail, naturally capped at one per reply an agent sends.

Deliberate behavior notes:

  • Machine-generated mail other than auto-replied is parked, including Auto-Submitted: auto-generated alert mail (monitoring systems, cron jobs). Any auto-generated sender is one sloppy autoresponder away from a loop with our acknowledgement, and a misfiring monitor floods the drawer instead of the ticket queue. If alert-to-ticket is ever needed, it should be an explicit account setting in a follow-up.
  • A threaded auto-reply is a customer communication, so it can flip a Replied/Resolved ticket's status back — the pre-existing behavior for any customer reply.

Testing

helpdesk/overrides/test_email_account.py — 15 tests:

  • 11 pure stdlib unit tests over auto_generated_reason and _failed_recipient with real message fixtures (Gmail DSN with a message/delivery-status part, out-of-office kept, quarantine alert parked, a DSN with no Return-Path, genuine customer replies, the Auto-Submitted: no cases).
  • 4 integration tests (skipped automatically outside a site): the non-IMAP Unhandled Email trace, and the ticket comment for a matched bounce, a matched auto-generated reply, and an unmatched bounce (no comment).

Also verified end to end on a dev site by driving get_inbound_mails with a stubbed mail client: the bounce is parked with reason delivery status notification and the ticket gets the delivery-failed comment with the failed address; the genuine customer reply in the same batch is kept.

Screenshots

The agent's reply bounces and the customer's out-of-office arrives, on the same ticket. The bounce is parked and leaves the internal comment; the out-of-office threads like a normal reply. Before this PR the bounce would have threaded in as a fake customer reply and started the loop; agents saw nothing about the failed delivery.

bounce comment and threaded out-of-office

Inbound mail was only screened with the X-Auto-Generated header, which
only helpdesk's own acks set. A real bounce or out-of-office therefore
reached ticket creation and could start three separate loops: the ack in
HD Ticket.after_insert replies to raised_by (the address that just
bounced), threading onto a portal ticket makes frappe CC the parent doc's
owner on every inbound mail, and an account with enable_auto_reply
answers mailer-daemon directly.

Detect machine-generated mail by the standard markers instead: RFC 3834
Auto-Submitted, the RFC 3464 multipart/report delivery-status type, and
the RFC 5321 null return-path. Matches are routed to handle_bad_emails so
they land in Unhandled Email rather than disappearing -- the fetch has
already marked them seen either way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.24088% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.30%. Comparing base (04ac2c4) to head (4592c40).
⚠️ Report is 295 commits behind head on develop.

Files with missing lines Patch % Lines
helpdesk/overrides/email_account.py 78.43% 11 Missing ⚠️
helpdesk/overrides/test_email_account.py 98.83% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3628      +/-   ##
===========================================
+ Coverage    68.57%   74.30%   +5.72%     
===========================================
  Files          140      149       +9     
  Lines         9181    11082    +1901     
===========================================
+ Hits          6296     8234    +1938     
+ Misses        2885     2848      -37     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@RitvikSardana
RitvikSardana marked this pull request as ready for review July 30, 2026 10:53
@mergify

mergify Bot commented Jul 30, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Reviews (7): Last reviewed commit: "chore: shorten comments" | Re-trigger Greptile

@aerodeval

Copy link
Copy Markdown
Member Author

@RitvikSardana have to test one more thing, do not merge yet

@aerodeval
aerodeval marked this pull request as draft August 7, 2026 13:02
Record Unhandled Email on every transport, not just IMAP, and leave an
internal comment on the matched ticket when a bounce or auto-reply is
parked, so agents still see failed deliveries and out-of-office replies.
@aerodeval
aerodeval marked this pull request as ready for review September 2, 2026 20:12
An out of office should reach agents on the ticket, and auto-replied
senders rate-limit themselves so they cannot sustain a loop the way
bounces and auto-generated feeds can. Those still get parked.
@aerodeval
aerodeval merged commit 9b1023c into frappe:develop Sep 3, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants